Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core: fix crash when computed nested map given in module block #13343

Merged
merged 2 commits into from
Apr 4, 2017

Conversation

apparentlymart
Copy link
Contributor

This crash resulted because the type switch checked for either of two types but the type assertion within it assumed only one of them.

A straightforward (if inelegant) fix is to simply duplicate the relevant case block and change the type assertion, thus allowing the types to match up in all cases.

This fixes #13297.

(Also includes a basic test for this eval type, since it entirely lacked one before.)

Copy link
Member

@jbardin jbardin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a minor code style comment, otherwise LGTM!

@@ -114,7 +114,6 @@ type EvalVariableBlock struct {
VariableValues map[string]interface{}
}

// TODO: test
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love seeing these go away

@@ -175,14 +174,22 @@ func (n *EvalVariableBlock) setUnknownVariableValueForPath(path string) error {
var current interface{} = n.VariableValues[pathComponents[0]]
for i := 1; i < len(pathComponents); i++ {
switch current.(type) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could define switch tCurrent := current.(type) { here, and get rid of the redundant assertions below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes, good point! I will update this before merging.

This crash resulted because the type switch checked for either of two
types but the type assertion within it assumed only one of them.

A straightforward (if inelegant) fix is to simply duplicate the relevant
case block and change the type assertion, thus allowing the types to match
up in all cases.

This fixes #13297.
This previously lacked tests altogether. This new test verifies the
"happy path", ensuring that both literal and computed values pass through
correctly into the VariableValues map.
@apparentlymart apparentlymart force-pushed the b-gh13297-nested-map-crash branch from acd225f to df4c0a0 Compare April 4, 2017 19:04
@apparentlymart apparentlymart merged commit 28d6d91 into master Apr 4, 2017
@grubernaut grubernaut deleted the b-gh13297-nested-map-crash branch May 11, 2017 00:32
@ghost
Copy link

ghost commented Apr 12, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash when passing nested hash to module containing resource reference
2 participants